@richard-cox added you as a reviewer not only because I couldn't find the exact root cause but because of the implications this might have on other areas and potencial checks to be done. Let me know if you want to get together to look into this

This looks like another instance where inheritAttrs: false would resolve the issue. I'm currently testing #11971 to see if there's a broader approach that we can take address this.

This looks like another instance where inheritAttrs: false would resolve the issue. I'm currently testing #11971 to see if there's a broader approach that we can take address this.

@rak-phillip I am interested in knowing more about the root cause of this.... I never understood why adding a root element would fix this 😛

@aalves08 this all has to do with how Vue3 manages fallthrough attributes¹ for root-level ancestor components - I've created a minimal repro to demonstrate the behavior:

https://play.vuejs.org/#eNqVUk1v2kAQ/SujvdBKEVbTnpCLRCkhVBQqWvW0F8ceg6nZ3e5HQmTx3zO7xjYoKCKSZXtm3uy+N/MqNlKq/+iQDVhsUl0oO+Si2CmpLYw3RZlBruUOev0oRB7a44IL3AdIhnniSgsVFwCppD6BwpoBVMfuw42vZIlNPnysUQAardOiiXxspNMpDqB3N5rN6XyfPfgPveiJo5YaBRZ3qkwsUgQQ19cMmjO+ctb8cgYRYeKobWA3zJpUirxY97dGChIdSHDmqRcl6qWyhRSGM1JQ0+AsKUv59CPkrHYYBIWeDab/LuS3Zu9znP0iJqgfiUhbs4leo63Lk98L3NN/W9zJzJWEfqO4Im2l8xxr2DcnMqJ9ggtsZ2GBhVj/MZO9RWEaUZ5omGvAc0brHL8hvaP7uf+l2QdNsfXCJdtMdSKyc+90qXcY6OSczkVwjY3aFEAuJdnqIdG944SOxrraXycsrjMZFzSgc8FnU7okXGmpvOZG2Csh9llRuHzYYmpbIRr/u0Jj1q31alWbT8P7yXy+hLvV8idMV6PF9/H9bDGNI6rUiNvh36R0CDLvCFH5NpSrqk3CIdx1qv/wAoIebws=

App.vue supplies :resource to Child.vue; since Child.vue doesn't declare a resource prop, resource is applied as an attribute to the root-level element (GrandChild), overwriting the :resource="resource" declaration.

This can be fixed in several ways:

• Adding a div wrapper to the child component, causing the resource attribute to be applied to the div instead (the change in this PR)
• Applying inheritAttrs: false to the child component, this will signal that we don't want to automatically apply fallthrough attributes to the root-level element
• Changing the name of the conflicting attribute (Rename resource to resourceType in ResourceDetail #11971); this will still apply a fallthrough attribute to the root-level element, but in a way that will avoid conflicts
• Use either v-bind.attr="$data" or v-bind.prop="$data" to prevent the fallthrough behavior²; this can allow us to define how we want fallthrough attributes to be applied from the parent

@aalves08 can you double check if adding inheritAttrs: false instead of a div wrapper fixes the issue? If so we should wait on the conclusion of #11971 and re-test after that merges

@richard-cox I can confirm that inheritAttrs: false instead of a div does indeed fix this. Will wait for #11971 to be merged and I'll re-test this. Thank you 🙏

@richard-cox #11971 did not fix this. Used inheritAttrs: false instead of a div and problem is fixed. Ready for re-review 🙏

@richard-cox #11971 did not fix this. Used inheritAttrs: false instead of a div and problem is fixed. Ready for re-review 🙏

hmm that gives me quite a bit of pause.. perhaps the solution implemented in #11971 is not the correct one for addressing this problem at the root. I'd like to investigate the issue here a little bit to better understand what else might be causing the issue.

I've narrowed this one down to the errors data property from the create-edit-view mixin:

This is a different manifestation of the same issue, fallthrough attributes for root-level ancestor components are overwriting props. I don't intend to block the merging of this PR and I think that the inheritAttrs: false approach is the right one to take to solve this problem.

I'm also going to investigate alternative solutions to see if we can prevent this issue from popping up in other components.

@richard-cox can this fix be approved and merged or should we wait for more feedback from @rak-phillip ?

There's something still very odd about this. If the error is down to a a mixin containing a data error it should also affect other similar places but it doesn't. I've made the edit experience for secrets and persistentvolumes at a high level match that of navlinks and they both correctly show errors caused by request failures.

The hierarchy is strange

• ResourceDefinition(grandparent)
  • contains CreateEditView mixin
    • contains data: { error }
  • contains the edit Component (parent)
    • contains CreateEditView mixin
      • contains data: { error }
    • contains CRUResource (child)
      • has an error prop
      • shows banner if has error

So errors is passed around in different ways. However isn't the same everywhere... in surprising ways

• ResourceDefinition - this.errors is always empty.
  • Kind of expected. it's own context of CreateEditView isn't making the http request
• edit component - this.errors is always populated
  • This is good, as it passes down to CRUResource via :errors="errors"
• CRUResource - this.errors populated for services, configmaps. NOT populated for navlinks

I would expect errors to come from ResourceDefinition (via mixin) --> edit component --> cruresouce and be broken everywhere, which isn't happening.

Basically looking for some clarification / comfort that the problem here isn't universal

@richard-cox this is explained by fallthrough attributes overwriting props in root-level ancestor components. in

In components like shell/components/ResourceDetail/index.vue, we bind the entire data property to dynamically rendered components

v-bind="$data" will attempt bind each property defined in data to the child component as a prop. The child component will handle and manage any matching props that are defined (in this PR, errors); any unmatched props in the child component will be applied to the root-level element of the child component via $attrs.

ui.cattle.io.navlink.vue does not define an errors prop in the component, nor is the errors prop defined by any of the mixins that it consumes. Therefore, errors will fallthrough to the root-level component within ui.cattle.io.navlink.vue, overwriting :errors="fvUnreportedValidationErrors" in the root-level component and causing the bug described in the issue

I think that the overall problem is two-fold:

1. A breaking change in behavior related to fallthrough attributes and root-level ancestor components - Vue2 did not overwrite props in root-level ancestors, while Vue3 does
2. Lack of a well-defined contract between shell/components/ResourceDetail/index.vue and the components it dynamically renders. Any given component that is dynamically rendered by ResourceDetail can experience conflict if it's not prepared to consume a data prop supplied to it by ResourceDetail.

The way that we have been addressing this issue is to add inheritAttrs: false to affected components. I think that this is the best approach that we have right now.

@rak-phillip I get the general failure, but trying to understand why it doesn't also make pages like persistentvolumes fail. Something to do with vue and the v-if/else?

secrets now seem clear (given cruresource is wrapped in a root level form element)

@richard-cox persistentvolumes already has inheritAttrs: false specified, which would explain why we don't see the issue with errors there

wrt to secrets, that's correct, the root-level form element will have $attrs applied by default.

Grand, that's all good now. Then we just need to run through the edit components and check where the root component is the cruresource that would receive the wrong errors. If there's no other we can get this in and add this to the vue3 extension docs guide. If there's lots we might want to consider a generic fix

Looks like this same issue appears in a number of different places (some aren't fixed by inheritAttrs: false,)

Standout - These need to be confirmed or fixed in 2.10.0

• shell/edit/management.cattle.io.user.vue
• shell/edit/auth/azuread.vue
• shell/edit/auth/ldap/index.vue
• shell/edit/auth/saml.vue
• shell/edit/auth/oidc.vue
• shell/edit/auth/googleoauth.vue
• shell/edit/auth/github.vue
• shell/edit/nodeDriver.vue
• shell/edit/kontainerDriver.vue

Debatable - These need to be confirmed but not maybe fixed in 2.10.0

• shell/edit/fleet.cattle.io.cluster.vue
• shell/edit/fleet.cattle.io.clustergroup.vue

Others - These need to be confirmed and fixed at some point

• shell/edit/cis.cattle.io.clusterscanbenchmark.vue
• shell/edit/helm.cattle.io.projecthelmchart.vue
• shell/edit/k8s.cni.cncf.io.networkattachmentdefinition.vue
• shell/edit/management.cattle.io.projectroletemplatebinding.vue
• shell/edit/monitoring.coreos.com.route.vue
• shell/edit/constraints.gatekeeper.sh.constraint/index.vue
• shell/edit/logging-flow/index.vue
• shell/edit/logging.banzaicloud.io.output/index.vue
• shell/edit/monitoring.coreos.com.alertmanagerconfig/index.vue
• shell/edit/monitoring.coreos.com.alertmanagerconfig/receiverConfig.vue
• shell/edit/monitoring.coreos.com.prometheusrule/index.vue
• shell/edit/monitoring.coreos.com.receiver/index.vue
• shell/edit/networking.istio.io.destinationrule/index.vue

Of Note

• shell/edit/provisioning.cattle.io.cluster/import.vue (this one matches pattern... but works without inheritAttrs)
• The hack was already put in a number of affected places by Fixed edit as yaml not working #11803

@rak-phillip i'll leave it up to you on how to split those up, and where it leaves this PR

@richard-cox I can work through each of the components listed and put together an issue to resolve any offenders. I don't see any reason why this PR should be blocked at this point.